Add an auto-generated unicode character category file#4605
Add an auto-generated unicode character category file#4605TheBlueMatt wants to merge 1 commit intolightningdevkit:mainfrom
Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
TheBlueMatt
force-pushed
the
2026-05-unicode-autogen
branch
from
b6f8c03 to
bd75483
Compare
| let is_other = is_unicode_general_category_other(c); | ||
| let is_unassigned = is_unicode_general_category_unassigned(c); | ||
| let c = if c.is_control() || is_other || is_unassigned { |
There was a problem hiding this comment.
Nit: c.is_control() is now fully redundant — it checks Cc (Control), which is already covered by is_unicode_general_category_other (see 0x0000..=0x001F and 0x007F..=0x009F in unicode.rs). The old code needed it because is_format_char only covered Cf, but the new function covers all of Cc / Cf / Cs / Co.
Not a bug (the || short-circuits harmlessly), but it's potentially confusing because it suggests is_other doesn't handle control characters.
| let is_other = is_unicode_general_category_other(c); | |
| let is_unassigned = is_unicode_general_category_unassigned(c); | |
| let c = if c.is_control() || is_other || is_unassigned { | |
| let c = if is_unicode_general_category_other(c) || is_unicode_general_category_unassigned(c) { |
Review SummaryNew issue found
Correction to prior reviewMy three prior comments about the YAML workflow file (missing Prior comment still applicable
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4605 +/- ##
==========================================
+ Coverage 86.09% 86.15% +0.06%
==========================================
Files 157 158 +1
Lines 108828 109323 +495
Branches 108828 109323 +495
==========================================
+ Hits 93694 94189 +495
+ Misses 12519 12518 -1
- Partials 2615 2616 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Alright, seems maybe a bit excessive, but fine by me. No need to check this on every CI run though, let's just move it to a weekly cronjob?
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Looks good, please squash.
|
Claude's reviews seem legit though |
1a01b5a added detection of unicode format characters in `PrintableString`, but used a hard-coded table which may eventually become out of date. Here we switch to an auto-generated table, include all `General_Category` `Other` characters, and also ban unallocated code points. Finally, CI validates that the file is kept up to date. Written by Claude
TheBlueMatt
force-pushed
the
2026-05-unicode-autogen
branch
from
2de5954 to
65e8cc8
Compare
|
Squashed and fixed claude's comments: $ git diff-tree -U2 2de5954d9 65e8cc8d5
diff --git a/.github/workflows/check_unicode.yml b/.github/workflows/check_unicode.yml
index c22a4e3b29..a01add3f81 100644
--- a/.github/workflows/check_unicode.yml
+++ b/.github/workflows/check_unicode.yml
@@ -5,4 +5,5 @@ on:
- cron: '42 3 * * *'
+jobs:
check-unicode:
runs-on: ubuntu-latest
@@ -10,4 +11,6 @@ on:
issues: write
steps:
+ - name: Checkout source code
+ uses: actions/checkout@v4
- name: Check unicode file state
env:
@@ -21,3 +24,3 @@ on:
BODY="The unicode character listing is out of date, see $RUN_URL"
gh issue create --title "$TITLE" --body "$BODY"
- fi
+ fi |
| TITLE="Unicode listing out of date: ${{ github.workflow }}" | ||
| RUN_URL="https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}" | ||
| BODY="The unicode character listing is out of date, see $RUN_URL" | ||
| gh issue create --title "$TITLE" --body "$BODY" |
There was a problem hiding this comment.
Bug: No deduplication — this creates a new issue every day the file is out of date. Unicode updates are infrequent (roughly annual), but if nobody regenerates the file promptly, this will spam the issue tracker with identical issues.
build.yml already has the right pattern (lines 380-391): check for an existing open issue with a label, and comment on it instead of creating a duplicate. This workflow should do the same.
Minimal fix sketch:
LABEL="unicode outdated"
gh label create "$LABEL" 2>/dev/null || true
EXISTING=$(gh issue list --label "$LABEL" --state open --json number --jq '.[0].number // empty')
if [ -n "$EXISTING" ]; then
gh issue comment "$EXISTING" --body "$BODY"
else
gh issue create --title "$TITLE" --label "$LABEL" --body "$BODY"
fi4 participants
1a01b5a added detection of unicode format characters in
PrintableString, but used a hard-coded table which may eventually become out of date.Here we switch to an auto-generated table, include all
General_CategoryOthercharacters, and also ban unallocated code points.Finally, CI validates that the file is kept up to date.
Written by Claude